Refactor interface generator#1114
Conversation
…placed by InterfaceGenerator + IInterfacePartitioning)
…fitGeneratorSettings fields
📝 WalkthroughWalkthroughThis PR refactors Refit interface code generation from a polymorphic inheritance pattern ( ChangesInterface Generation Architecture Refactor
Command, Tests, and Test Artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1114 +/- ##
==========================================
- Coverage 95.33% 87.37% -7.96%
==========================================
Files 28 30 +2
Lines 3021 2472 -549
==========================================
- Hits 2880 2160 -720
- Misses 45 219 +174
+ Partials 96 93 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/Refitter.Core/InterfaceGenerator.cs (1)
361-423: ⚖️ Poor tradeoffConsider refactoring
GenerateHeadersto reduce cognitive complexity.SonarCloud flags this method with cognitive complexity of 30 (allowed: 15). The method handles three distinct concerns: Accept headers, Content-Type headers, and Authorization headers. Consider extracting each concern into separate helper methods.
♻️ Suggested refactoring approach
private void GenerateHeaders( OpenApiOperation operation, CSharpOperationModel operationModel, StringBuilder code) { var headers = new List<string>(); + + AddAcceptHeaders(operation, headers); + AddContentTypeHeaders(operation, operationModel, headers); + AddAuthorizationHeaders(operationModel, headers); - if (settings.AddAcceptHeaders && document.SchemaType is >= NJsonSchema.SchemaType.OpenApi3) - { - // ... existing Accept header logic ... - } - - if (settings.AddContentTypeHeaders && document.SchemaType is >= NJsonSchema.SchemaType.OpenApi3) - { - // ... existing Content-Type logic ... - } - - if (settings.AuthenticationHeaderStyle == AuthenticationHeaderStyle.Method) - { - // ... existing Authorization logic ... - } - if (headers.Any()) { code.AppendLine($"{Separator}{Separator}[Headers({string.Join(", ", headers)})]"); } } + +private void AddAcceptHeaders(OpenApiOperation operation, List<string> headers) { /* ... */ } +private void AddContentTypeHeaders(OpenApiOperation operation, CSharpOperationModel operationModel, List<string> headers) { /* ... */ } +private void AddAuthorizationHeaders(CSharpOperationModel operationModel, List<string> headers) { /* ... */ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Core/InterfaceGenerator.cs` around lines 361 - 423, The GenerateHeaders method is too complex; extract the three concerns into helpers: Create GenerateAcceptHeaders(OpenApiOperation operation, CSharpOperationModel operationModel) to compute and return any Accept header strings (preserve the document.SchemaType check and uniqueContentTypes logic), Create GenerateContentTypeHeader(OpenApiOperation operation, CSharpOperationModel operationModel) to return the Content-Type header (preserve requestBody content selection and multipart/form-data skip), and CreateAuthorizationHeaders(CSharpOperationModel operationModel) to return Authorization header strings (preserve AuthenticationHeaderStyle.Method, settings.SecurityScheme filtering and document.SecurityDefinitions lookup and bearer-scheme check); then simplify GenerateHeaders to call these helpers, aggregate their returned headers into the headers list, and keep the final AppendLine behavior unchanged so functionality and header formatting remain identical.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Line 46: Update the AGENTS.md smoke-test description to reflect actual
behavior: note that test/smoke-tests.ps1 only publishes
src/Refitter/Refitter.csproj from source when not run with -UseDocker and not in
-UseProduction, that generated clients are compiled against ./ConsoleApp/... (CI
runs with working-directory: test so that resolves to test/ConsoleApp/), and
that test/OpenAPI/v3.4 is not present so the v3.4 client generation loop is
skipped rather than producing clients; reference test/smoke-tests.ps1,
Refitter.csproj, -UseDocker, -UseProduction, ConsoleApp, and test/OpenAPI/v3.4
in the updated sentence.
In `@src/Refitter.Core/ByEndpointInterfacePartitioning.cs`:
- Around line 26-27: GetDynamicQuerystringParameterType currently does
interfaceName.Substring(1).Replace("Endpoint","QueryParams") and can return an
empty name when baseOperationName is empty (InterfaceGenerator sets
baseOperationName = string.Empty for no operations). Add a guard in
GetDynamicQuerystringParameterType to detect an empty/too-short interfaceName
(e.g., interfaceName == "I" or interfaceName.Length <= 1) and in that case
return methodName + "QueryParams" as the fallback; otherwise keep the existing
substring+replace behavior so you never produce an empty dynamic query type.
In `@src/Refitter.Core/InterfaceGenerator.cs`:
- Around line 32-69: The single-interface path currently passes all operations
to GenerateSingleInterface which can produce an empty interface when all ops are
deprecated; mirror the multi-interface behavior by filtering operations first
using settings.GenerateDeprecatedOperations and op.Operation.IsDeprecated (same
logic used in the multi path), and only call GenerateSingleInterface when the
resulting nonDeprecatedOperations list has at least one item; keep passing
partitioning, title and knownInterfaceIdentifiers unchanged so naming logic and
side effects remain consistent.
---
Nitpick comments:
In `@src/Refitter.Core/InterfaceGenerator.cs`:
- Around line 361-423: The GenerateHeaders method is too complex; extract the
three concerns into helpers: Create GenerateAcceptHeaders(OpenApiOperation
operation, CSharpOperationModel operationModel) to compute and return any Accept
header strings (preserve the document.SchemaType check and uniqueContentTypes
logic), Create GenerateContentTypeHeader(OpenApiOperation operation,
CSharpOperationModel operationModel) to return the Content-Type header (preserve
requestBody content selection and multipart/form-data skip), and
CreateAuthorizationHeaders(CSharpOperationModel operationModel) to return
Authorization header strings (preserve AuthenticationHeaderStyle.Method,
settings.SecurityScheme filtering and document.SecurityDefinitions lookup and
bearer-scheme check); then simplify GenerateHeaders to call these helpers,
aggregate their returned headers into the headers list, and keep the final
AppendLine behavior unchanged so functionality and header formatting remain
identical.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: de34e959-dffd-4be3-a39f-1fdc42b128ec
📒 Files selected for processing (21)
AGENTS.mdsrc/Refitter.Core/ByEndpointInterfacePartitioning.cssrc/Refitter.Core/ByTagInterfacePartitioning.cssrc/Refitter.Core/IInterfacePartitioning.cssrc/Refitter.Core/IRefitInterfaceGenerator.cssrc/Refitter.Core/InterfaceGenerator.cssrc/Refitter.Core/OpenApiOperationInfo.cssrc/Refitter.Core/RefitGenerator.cssrc/Refitter.Core/RefitInterfaceGenerator.cssrc/Refitter.Core/RefitMultipleInterfaceByTagGenerator.cssrc/Refitter.Core/RefitMultipleInterfaceGenerator.cssrc/Refitter.Core/SingleInterfacePartitioning.cssrc/Refitter.Core/XmlDocumentationGenerator.cssrc/Refitter.Tests/Apizr/ApizrGeneratorWithMicrosoftHttpResilienceTests.cssrc/Refitter.Tests/Apizr/ApizrGeneratorWithPollyTests.cssrc/Refitter/GenerateCommand.cstest/OpenAPI/v3.0/bot.components.yamltest/OpenAPI/v3.0/bot.paths.yamltest/OpenAPI/v3.0/hubspot-events.jsontest/OpenAPI/v3.0/hubspot-webhooks.jsontest/smoke-tests.ps1
💤 Files with no reviewable changes (8)
- src/Refitter.Core/IRefitInterfaceGenerator.cs
- test/OpenAPI/v3.0/hubspot-events.json
- src/Refitter.Core/RefitMultipleInterfaceByTagGenerator.cs
- test/OpenAPI/v3.0/hubspot-webhooks.json
- test/OpenAPI/v3.0/bot.paths.yaml
- src/Refitter.Core/RefitMultipleInterfaceGenerator.cs
- test/OpenAPI/v3.0/bot.components.yaml
- src/Refitter.Core/RefitInterfaceGenerator.cs
|
|
||
| - Build workflow: `.github/workflows/build.yml` | ||
| - Smoke tests: `.github/workflows/smoke-tests.yml` runs `test/smoke-tests.ps1` | ||
| - The smoke test script publishes the CLI from source, generates clients against dozens of OpenAPI specs (v2.0, v3.0, v3.1, v3.4), and compiles them against console apps in `test/ConsoleApp/`. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect smoke-tests.ps1 to verify claimed behavior
# Check if script exists and contains expected operations
test -f test/smoke-tests.ps1 && echo "✓ Smoke test script found" || echo "✗ Smoke test script NOT found"
# Look for key operations
echo "=== Checking for CLI publishing step ==="
grep -n "publish\|dotnet publish" test/smoke-tests.ps1 || echo "No publish command found"
echo "=== Checking for multi-spec generation ==="
grep -n "v2.0\|v3.0\|v3.1\|v3.4\|spec" test/smoke-tests.ps1 | head -10
echo "=== Checking for ConsoleApp reference ==="
grep -n "ConsoleApp" test/smoke-tests.ps1Repository: christianhelle/refitter
Length of output: 2072
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="test/smoke-tests.ps1"
[ -f "$FILE" ] || { echo "Missing $FILE"; exit 1; }
echo "== Versions present =="
rg -n "v3\.1|v3\.4|v3\.0|v2\.0" "$FILE" || true
echo
echo "== ConsoleApp / path usage =="
rg -n "test/ConsoleApp|ConsoleApp" "$FILE" || true
echo
echo "== Generation loops / spec collection context =="
# Show the most relevant sections without dumping the whole file
# (covers the loop that collects generation tasks and likely where spec versions are enumerated)
start=$(rg -n "Collect generation tasks" "$FILE" | head -n1 | cut -d: -f1)
if [ -n "${start:-}" ]; then
s=$((start-40)); e=$((start+140))
sed -n "${s},${e}p" "$FILE" | cat -n
fi
echo
echo "== Client generation / compilation context =="
# Show around the BuildSolution -smokeTest calls and any surrounding generation/install steps
rg -n "smokeTest" "$FILE" | head -n 10 | sed -n '1,10p'
for line in $(rg -n "BuildSolution -solution \"\./ConsoleApp/.*\".*-smokeTest" "$FILE" | head -n 5 | cut -d: -f1); do
s=$((line-30)); e=$((line+60))
echo "--- around line $line ---"
sed -n "${s},${e}p" "$FILE" | cat -n
done
echo
echo "== Count of generated spec files (heuristic) =="
# Try to estimate “dozens” by counting distinct spec paths that the script considers for generation.
# This looks for Test-Path checks and then counts unique SpecPath assignments referencing OpenAPI.
# (If the script uses globbing dynamically, this may undercount; still useful.)
rg -n "SpecPath = .*OpenAPI" "$FILE" || true
echo "Done."Repository: christianhelle/refitter
Length of output: 33445
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="test/smoke-tests.ps1"
echo "== Context around dotnet publish =="
line_pub=$(rg -n "dotnet publish|Refitter\.csproj -c Release -o bin -f net10\.0" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -n "${line_pub}" ]; then
s=$((line_pub-30)); e=$((line_pub+40))
sed -n "${s},${e}p" "$FILE" | cat -n
else
echo "No publish match found (unexpected)."
fi
echo
echo "== Find filename arrays for v2.0/v3.0/v3.1/v3.4 =="
rg -n "\\$filenames\\s*=|\\$v31Filenames\\s*=|\\$v34WebhookFilenames\\s*=" "$FILE" || true
echo
for var in "filenames" "v31Filenames" "v34WebhookFilenames"; do
echo "== Context for \$$var =="
match_line=$(rg -n "\\$${var}\\s*=" "$FILE" | head -n1 | cut -d: -f1 || true)
if [ -z "${match_line}" ]; then
echo "Not found: \$$var"
continue
fi
s=$((match_line-10)); e=$((match_line+120))
sed -n "${s},${e}p" "$FILE" | cat -n
echo
done
echo "== Quick count heuristic (number of filenames elements) =="
# Extract simple list items from array literals like @("a","b",...) / @("a", "b")
python3 - <<'PY'
import re, pathlib
p=pathlib.Path("test/smoke-tests.ps1")
t=p.read_text(encoding="utf-8", errors="ignore")
def extract(var):
# capture @(... ) after $var =
m=re.search(rf"\${var}\s*=\s*@\(([\s\S]*?)\)\s*[\r\n]", t)
if not m:
return None
block=m.group(1)
items=re.findall(r'"([^"]+)"', block)
return items
for var in ["filenames","v31Filenames","v34WebhookFilenames"]:
items=extract(var)
if items is None:
print(var, "=> not an @(... ) literal / not easily parseable")
else:
print(var, "=>", len(items), "entries")
PYRepository: christianhelle/refitter
Length of output: 5011
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="test/smoke-tests.ps1"
echo "== ConsoleApp path existence checks =="
ls -d test/ConsoleApp ./test/ConsoleApp 2>/dev/null || true
ls -d ConsoleApp ./ConsoleApp 2>/dev/null || true
ls -d test/ConsoleApp/ConsoleApp.slnx 2>/dev/null || true
ls -d test/ConsoleApp/ConsoleApp.Core.slnx 2>/dev/null || true
ls -d ConsoleApp/ConsoleApp.slnx 2>/dev/null || true
ls -d ConsoleApp/ConsoleApp.Core.slnx 2>/dev/null || true
echo
echo "== Locate filename arrays in test/smoke-tests.ps1 =="
rg -n '\$filenames\s*=|\\$v31Filenames\s*=|\\$v34WebhookFilenames\s*=' "$FILE" || true
rg -n '\$filenames\b|\$v31Filenames\b|\$v34WebhookFilenames\b' "$FILE" | head -n 50
echo
echo "== Parse array lengths from literals (heuristic) =="
python3 - <<'PY'
import re, pathlib
p=pathlib.Path("test/smoke-tests.ps1")
t=p.read_text(encoding="utf-8", errors="ignore")
def extract(var):
# capture @(... ) after $var =
m=re.search(rf"\${var}\s*=\s*@\(([\s\S]*?)\)\s*[\r\n]", t)
if not m:
return None
block=m.group(1)
# collect string literals inside the array
items=re.findall(r'"([^"]+)"', block)
return items
for var in ["filenames","v31Filenames","v34WebhookFilenames"]:
items=extract(var)
if items is None:
print(f"{var}: not found as an @(... ) literal")
else:
print(f"{var}: {len(items)} entries")
# show first/last few for sanity
if len(items)>0:
print(" first:", items[:5])
print(" last :", items[-5:])
PYRepository: christianhelle/refitter
Length of output: 947
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="test/smoke-tests.ps1"
echo "== File header =="
sed -n '1,120p' "$FILE" | cat -n
echo
echo "== Look for working-directory / script-root handling =="
rg -n "Set-Location|\$PSScriptRoot|\$PWD|Push-Location|WorkingDirectory|cd " "$FILE" || true
echo
echo "== Where RunTests is invoked with BuildFromSource =="
rg -n "RunTests\s*-" "$FILE" || true
sed -n '520,660p' "$FILE" | cat -nRepository: christianhelle/refitter
Length of output: 9729
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find OpenAPI dir(s)
echo "== OpenAPI directories (by name) =="
fd -td d -a "OpenAPI" . | head -n 50 || true
# Prefer test/OpenAPI if present, else first match
OPENAPI_DIR="$(ls -d test/OpenAPI 2>/dev/null | head -n1 || true)"
if [ -z "${OPENAPI_DIR:-}" ]; then
OPENAPI_DIR="$(fd -td d -a "OpenAPI" . | head -n1)"
fi
echo
echo "== Using OpenAPI dir =="
echo "$OPENAPI_DIR"
echo
echo "== Count existing spec files per version/format =="
for ver in v2.0 v3.0 v3.1 v3.4; do
if [ -d "$OPENAPI_DIR/$ver" ]; then
c_json=$(find "$OPENAPI_DIR/$ver" -type f -name "*.json" | wc -l)
c_yaml=$(find "$OPENAPI_DIR/$ver" -type f -name "*.yaml" -o -name "*.yml" | wc -l)
echo "$ver: json=$c_json yaml/yml=$c_yaml"
else
echo "$ver: missing"
fi
done
echo
echo "== Total spec files for those versions (json/yaml) =="
total=0
for ver in v2.0 v3.0 v3.1 v3.4; do
c=$(find "$OPENAPI_DIR/$ver" -type f \( -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) 2>/dev/null | wc -l)
total=$((total + c))
done
echo "total=$total"Repository: christianhelle/refitter
Length of output: 1165
🏁 Script executed:
#!/bin/bash
set -euo pipefail
OPENAPI_DIR="test/OpenAPI"
echo "== test/OpenAPI tree (depth 3) =="
# show just the directory/file names without huge output
find "$OPENAPI_DIR" -maxdepth 3 -type d | sort | sed 's|^./||' || true
echo
echo "== test/OpenAPI version dirs =="
ls -1 "$OPENAPI_DIR" 2>/dev/null || true
echo
echo "== Check for v3.4 webhook-example files =="
ls -1 "$OPENAPI_DIR/v3.4" 2>/dev/null || true
# find the exact filename referenced by the script: webhook-example.<json|yaml>
find "$OPENAPI_DIR" -type f \( -name "webhook-example.json" -o -name "webhook-example.yaml" -o -name "webhook-example.yml" \) | sed 's|^./||' || true
echo
echo "== Count all spec files under test/OpenAPI =="
find "$OPENAPI_DIR" -type f \( -name "*.json" -o -name "*.yaml" -o -name "*.yml" \) | wc -lRepository: christianhelle/refitter
Length of output: 388
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find how smoke-tests.ps1 is invoked =="
rg -n "smoke-tests\.ps1" . || true
echo
echo "== Inspect RunTests definition + parameters =="
rg -n "function RunTests|RunTests\s*\{" test/smoke-tests.ps1 || true
# show around the first match (usually contains parameter list)
ln=$(rg -n "function RunTests|RunTests\s*\{" test/smoke-tests.ps1 | head -n1 | cut -d: -f1)
if [ -n "$ln" ]; then
s=$((ln-40)); e=$((ln+120))
sed -n "${s},${e}p" test/smoke-tests.ps1 | cat -n
fi
echo
echo "== Search for Set-Location / PSScriptRoot anywhere in file =="
rg -n "Set-Location|\\$PSScriptRoot|Push-Location|Pop-Location|cd " test/smoke-tests.ps1 || trueRepository: christianhelle/refitter
Length of output: 9924
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,120p' test/smoke-tests.sh | cat -n
rg -n "workdir|cd test|Set-Location|WorkingDirectory|smoke-tests\.ps1" .github/workflows/smoke-tests.yml .github/workflows -S || true
sed -n '1,160p' .github/workflows/smoke-tests.yml | cat -n || trueRepository: christianhelle/refitter
Length of output: 2973
Fix smoke-test behavior documented for v3.4 (and CLI publish is conditional).
test/smoke-tests.ps1publishessrc/Refitter/Refitter.csprojfrom source only when not running with-UseDockerand not in-UseProduction; in those modes it won’t do the source publish step.- It does compile generated code against
./ConsoleApp/...(CI runs the script withworking-directory: test, so this resolves totest/ConsoleApp/), and it generates many v2.0/v3.0/v3.1 specs (there’stest/OpenAPI/v2.0,v3.0,v3.1), buttest/OpenAPI/v3.4doesn’t exist—so the v3.4 client generation loop will skip entirely.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@AGENTS.md` at line 46, Update the AGENTS.md smoke-test description to reflect
actual behavior: note that test/smoke-tests.ps1 only publishes
src/Refitter/Refitter.csproj from source when not run with -UseDocker and not in
-UseProduction, that generated clients are compiled against ./ConsoleApp/... (CI
runs with working-directory: test so that resolves to test/ConsoleApp/), and
that test/OpenAPI/v3.4 is not present so the v3.4 client generation loop is
skipped rather than producing clients; reference test/smoke-tests.ps1,
Refitter.csproj, -UseDocker, -UseProduction, ConsoleApp, and test/OpenAPI/v3.4
in the updated sentence.
| public string GetDynamicQuerystringParameterType(string interfaceName, string methodName) => | ||
| interfaceName.Substring(1).Replace("Endpoint", "QueryParams"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for uses of GetDynamicQuerystringParameterType and verify baseOperationName is never empty
rg -nP -C5 'GetDynamicQuerystringParameterType|GetBaseOperationName' --type=csRepository: christianhelle/refitter
Length of output: 8406
Guard against empty baseOperationName to avoid returning an empty dynamic query type
In src/Refitter.Core/ByEndpointInterfacePartitioning.cs (lines 26-27), GetDynamicQuerystringParameterType uses interfaceName.Substring(1) ...; if baseOperationName is empty, interfaceName can become "I" and this expression returns "" (no exception, but an empty generated type name). InterfaceGenerator explicitly sets baseOperationName to string.Empty when operations.Count == 0. Add a length/empty guard and fall back to a non-empty type name (e.g., methodName + "QueryParams" like the other partitionings).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Refitter.Core/ByEndpointInterfacePartitioning.cs` around lines 26 - 27,
GetDynamicQuerystringParameterType currently does
interfaceName.Substring(1).Replace("Endpoint","QueryParams") and can return an
empty name when baseOperationName is empty (InterfaceGenerator sets
baseOperationName = string.Empty for no operations). Add a guard in
GetDynamicQuerystringParameterType to detect an empty/too-short interfaceName
(e.g., interfaceName == "I" or interfaceName.Length <= 1) and in that case
return methodName + "QueryParams" as the fallback; otherwise keep the existing
substring+replace behavior so you never produce an empty dynamic query type.
| public IEnumerable<GeneratedCode> Generate(IInterfacePartitioning partitioning) | ||
| { | ||
| var operations = document.Paths | ||
| .SelectMany(path => path.Value, (path, op) => new OpenApiOperationInfo(path.Key, op.Key, op.Value)) | ||
| .ToList(); | ||
|
|
||
| var groups = operations | ||
| .GroupBy(op => partitioning.GetGroupKey(op)) | ||
| .ToList(); | ||
|
|
||
| var knownInterfaceIdentifiers = new HashSet<string>(); | ||
| var title = settings.Naming.UseOpenApiTitle && !string.IsNullOrWhiteSpace(document.Info?.Title) | ||
| ? document.Info!.Title.Sanitize() | ||
| : settings.Naming.InterfaceName; | ||
|
|
||
| if (partitioning.IsSingleInterface) | ||
| { | ||
| var singleGroup = groups.Count > 0 ? groups[0].ToList() : new List<OpenApiOperationInfo>(); | ||
| yield return GenerateSingleInterface(singleGroup, partitioning, title, knownInterfaceIdentifiers); | ||
| } | ||
| else | ||
| { | ||
| foreach (var group in groups) | ||
| { | ||
| var nonDeprecatedOperations = group | ||
| .Where(op => settings.GenerateDeprecatedOperations || !op.Operation.IsDeprecated) | ||
| .ToList(); | ||
|
|
||
| if (nonDeprecatedOperations.Count == 0) | ||
| continue; | ||
|
|
||
| foreach (var generatedCode in GenerateMultipleInterface(nonDeprecatedOperations, partitioning, title, knownInterfaceIdentifiers)) | ||
| { | ||
| yield return generatedCode; | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Inconsistent handling of all-deprecated operation groups between single and multi-interface modes.
In single interface mode (lines 47-51), all operations are passed to GenerateSingleInterface, which may generate an empty interface if all operations are deprecated and GenerateDeprecatedOperations is false (filtering happens per-method in GenerateMethod lines 153-156).
In multi-interface mode (lines 56-61), entire groups containing only deprecated operations are skipped with continue, preventing empty interface generation.
This inconsistency means single-interface mode could produce an interface with no methods when all operations are deprecated, while multi-interface mode would skip such groups entirely.
🔧 Proposed fix to skip empty interface in single mode
if (partitioning.IsSingleInterface)
{
var singleGroup = groups.Count > 0 ? groups[0].ToList() : new List<OpenApiOperationInfo>();
+ var nonDeprecatedOperations = singleGroup
+ .Where(op => settings.GenerateDeprecatedOperations || !op.Operation.IsDeprecated)
+ .ToList();
+
+ if (nonDeprecatedOperations.Count == 0 && singleGroup.Count > 0)
+ {
+ // All operations are deprecated and we're not generating them
+ yield break;
+ }
+
yield return GenerateSingleInterface(singleGroup, partitioning, title, knownInterfaceIdentifiers);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/Refitter.Core/InterfaceGenerator.cs` around lines 32 - 69, The
single-interface path currently passes all operations to GenerateSingleInterface
which can produce an empty interface when all ops are deprecated; mirror the
multi-interface behavior by filtering operations first using
settings.GenerateDeprecatedOperations and op.Operation.IsDeprecated (same logic
used in the multi path), and only call GenerateSingleInterface when the
resulting nonDeprecatedOperations list has at least one item; keep passing
partitioning, title and knownInterfaceIdentifiers unchanged so naming logic and
side effects remain consistent.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Refitter.Core/RefitGenerator.cs (1)
156-156: ⚡ Quick winReplace
varwith explicit type for guideline compliance.Use explicit typing here to align with the repository
.editorconfigrule that discouragesvarin C# files.Suggested change
- var interfaceGenerator = new InterfaceGenerator(settings, document, generator, docGenerator); + InterfaceGenerator interfaceGenerator = new(settings, document, generator, docGenerator);As per coding guidelines:
**/*.{cs,csx}discouragesvarusage (csharp_style_var_* = false:silent).Also applies to: 213-213
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Refitter.Core/RefitGenerator.cs` at line 156, Replace implicit var declarations with the explicit type InterfaceGenerator: change the declaration of interfaceGenerator where InterfaceGenerator is constructed (and the other occurrence noted around line 213) to use the concrete type name instead of var to comply with the repository's csharp_style_var_* rule; update both instantiations of InterfaceGenerator to use "InterfaceGenerator interfaceGenerator = new InterfaceGenerator(...)" (preserve constructor arguments and surrounding logic).Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/Refitter.Core/RefitGenerator.cs`:
- Line 156: Replace implicit var declarations with the explicit type
InterfaceGenerator: change the declaration of interfaceGenerator where
InterfaceGenerator is constructed (and the other occurrence noted around line
213) to use the concrete type name instead of var to comply with the
repository's csharp_style_var_* rule; update both instantiations of
InterfaceGenerator to use "InterfaceGenerator interfaceGenerator = new
InterfaceGenerator(...)" (preserve constructor arguments and surrounding logic).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 69358548-5f85-444e-ad19-c15a28d44b1c
📒 Files selected for processing (4)
src/Refitter.Core/ByEndpointInterfacePartitioning.cssrc/Refitter.Core/ByTagInterfacePartitioning.cssrc/Refitter.Core/InterfaceGenerator.cssrc/Refitter.Core/RefitGenerator.cs
🚧 Files skipped from review as they are similar to previous changes (3)
- src/Refitter.Core/ByTagInterfacePartitioning.cs
- src/Refitter.Core/ByEndpointInterfacePartitioning.cs
- src/Refitter.Core/InterfaceGenerator.cs
|



Closes #1123
Summary by CodeRabbit
Documentation
Refactor
Tests